Skip to content

docs better indicate that state updaters shallowly merge with state#9554

Merged
sophiebits merged 1 commit intofacebook:masterfrom
nsfmc:setStateDocsUpdate
May 1, 2017
Merged

docs better indicate that state updaters shallowly merge with state#9554
sophiebits merged 1 commit intofacebook:masterfrom
nsfmc:setStateDocsUpdate

Conversation

@nsfmc
Copy link
Copy Markdown
Contributor

@nsfmc nsfmc commented Apr 28, 2017

this was a surprise to me because the docs seemed to indicate that when
using an updater, the result needed to be a new state object. I was
not alone
i think in discovering this as a result of the previous tweet in the
thread.

i'm open to wording changes and such. i debated the final sentence reading:

The output of the updater is shallowly merged with prevState allowing updaters to return only the subset of state they want to change.

but it felt awkward. i think the other changes get the point across anyway, but open to suggestions.

this was a surprise to me because the docs seemed to indicate that when
using an updater, the result _needed_ to be a new state object. I was
[not alone](https://twitter.com/ken_wheeler/status/857939690191806464)
i think in discovering this as a result of the previous tweet in the
thread.
@sophiebits
Copy link
Copy Markdown
Collaborator

Thanks @nsfmc!

gaearon pushed a commit that referenced this pull request May 1, 2017
…9554)

this was a surprise to me because the docs seemed to indicate that when
using an updater, the result _needed_ to be a new state object. I was
[not alone](https://twitter.com/ken_wheeler/status/857939690191806464)
i think in discovering this as a result of the previous tweet in the
thread.
(cherry picked from commit f737d63)
flarnie pushed a commit that referenced this pull request Jun 12, 2017
…9554)

this was a surprise to me because the docs seemed to indicate that when
using an updater, the result _needed_ to be a new state object. I was
[not alone](https://twitter.com/ken_wheeler/status/857939690191806464)
i think in discovering this as a result of the previous tweet in the
thread.
(cherry picked from commit f737d63)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants